feat(dataset): add collect_all_keys option and keep CSV export columns aligned - #2079
Conversation
Mantisus
left a comment
There was a problem hiding this comment.
Thank you for finding this bug. However, we cannot use extrasaction=‘ignore’
There was a problem hiding this comment.
Pull request overview
This PR fixes CSV export for Dataset items that don’t share identical key order/sets, preventing values from being written under the wrong column when exporting heterogeneous items.
Changes:
- Update
export_csv_to_streamto write rows by key (viacsv.DictWriter) instead of positionalitem.values(). - Add regression tests covering heterogeneous item keys/order and skipping empty items during header selection.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/crawlee/_utils/file.py |
Switches CSV export implementation to key-based writing and changes how CSV headers are derived. |
tests/unit/_utils/test_file.py |
Adds regression tests for column alignment with heterogeneous items and for skipping empty mappings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
`export_csv_to_stream` built the header from the first item's keys but then wrote every row positionally with `csv.writer` and `item.values()`. Since `Dataset.push_data` accepts arbitrary mappings per item with no schema, items whose keys differ in membership or order from the first item had their values written under the wrong columns (or shifted into columns that do not exist), silently corrupting the CSV export of any non-uniform dataset. Write rows with `csv.DictWriter` keyed on the first item's fields and `extrasaction='ignore'`, so each value lands in its own column and unexpected keys are dropped. This matches the column-aligned behavior of Crawlee for JavaScript. Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
5b95959 to
bde4ba2
Compare
|
pushed a revision addressing the latest review and ci feedback. |
|
Same as here #2074 (comment)
|
vdusek
left a comment
There was a problem hiding this comment.
A few important things and then a few nits.
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
|
@vdusek addressed all new threads, including js-default parity, streaming behavior, bounded default memory, and value preservation tests. rebased and re-requesting review. thank you. |
…eagerly Address review findings on the CSV export rewrite: - Warn once per export, naming every key dropped because it is absent from the first item, instead of losing those values silently. Master wrote them to the wrong column, which was wrong but at least visible. - Build a throwaway writer up front so invalid options (e.g. `delimiter='ab'`) raise even when the dataset is empty. Master validated eagerly; the lazy writer made a misconfigured export succeed on a run that scraped nothing and fail on the next one. - Pass `extrasaction='ignore'` on the buffered path too, so both paths construct the writer identically and narrowing `fieldnames` later cannot raise mid-export. - Declare `restval`, which already worked but was rejected by the type checker at every call site, and correct the `ExportDataCsvWriterKwargs` docstring that named `csv.DictWriter` while listing only `csv.writer` options. - Cover the empty-items guard, the `if item` filter on the buffered path, and `collect_all_keys` through `Dataset.export_to` and `BasicCrawler.export_data`, none of which had a test. - Document the column behavior and `collect_all_keys` in the dataset export example.
|
Summary:
✍️ Drafted by Claude Code |
collect_all_keys option and keep CSV export columns aligned
collect_all_keys option and keep CSV export columns alignedcollect_all_keys option and keep CSV export columns aligned
csv.writer, so any item whose key set or key order differed from the first item's had its valuesshifted or placed under the wrong header. Rows are now written by key with
csv.DictWriter.instead of being appended out of position, and an item missing a column now gets an empty cell
instead of a short row. Exports of datasets with mixed keys therefore change content.
silently, so this is a Python-only addition.
collect_all_keysparameter toDataset.export_toandBasicCrawler.export_data. WhenTrue, the columns are the union of all item keys in first-seen order, so nothing is dropped. It
reads the whole dataset before writing the header, unlike the default. Named and behaving like
collectAllKeysin Crawlee for JS.restval, the value written for columns an item has no key for. It was rejected before,because
csv.writerhas no such argument.ExportDataCsvKwargsas mirroringcsv.DictWriterandExportDataJsonKwargsasmirroring
json.dump, so Crawlee-specific options stay out of them.collectAllKeysall matchDataset.exportToinpackages/core/src/storages/dataset.ts.✍️ Drafted by Claude Code